Skip to content

feat: Added new component six-breadcrumbs - #380

Merged
fortesp merged 32 commits into
mainfrom
feat/add-breadcrumbs-component
Sep 12, 2025
Merged

feat: Added new component six-breadcrumbs#380
fortesp merged 32 commits into
mainfrom
feat/add-breadcrumbs-component

Conversation

@fortesp

@fortesp fortesp commented Jun 18, 2025 •
edited
Loading

Copy link
Copy Markdown
Collaborator

🔗 Linked issue

❓ Type of change

  • 📖 Documentation (updates to the documentation, readme or JSdoc annotations)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • 👌 Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

📚 Description

New breadcrumbs component

📝 Checklist

  • I have linked an issue or discussion.
  • It's submitted to the main branch
  • When resolving a specific issue, it's referenced in the PR's title (e.g. fix #xxx[,#xxx], where "xxx" is the issue number)
  • I have updated the documentation accordingly.
  • All tests are passing
  • New/updated tests are included
  • I have updated the "upcoming" section inside docs/changelog.md explaining the changes I contributed

If adding a new feature, the PR's description includes:

  • A convincing reason for adding this feature (to avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it)

Other information:

@fortesp
fortesp requested review from colinscz and ibirrer June 18, 2025 14:08
@fortesp fortesp self-assigned this Jun 18, 2025
colinscz
colinscz requested changes Jun 19, 2025
View reviewed changes

@colinscz colinscz left a comment •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution, I made some comments and suggestions :)

Please double check regarding the documentation, we always want to show at least 1 example per component in the docs.

If you want to you can also implement an example usage of these components in one of the apps - up to you :)

Comment thread docs/components/six-breadcrumbs-item.md
Comment thread docs/changelog.md Outdated
Comment thread libraries/ui-library/src/components/six-breadcrumbs-item/six-breadcrumbs-item.tsx Outdated
Comment thread libraries/ui-library/src/components/six-breadcrumbs-item/six-breadcrumbs-item.tsx Outdated
Comment thread libraries/ui-library/src/components/six-breadcrumbs-item/six-breadcrumbs-item.tsx Outdated
Comment thread libraries/ui-library/src/components/six-breadcrumbs/six-breadcrumbs.scss Outdated
ibirrer
ibirrer requested changes Jun 19, 2025
View reviewed changes
Comment thread libraries/ui-library/src/components/six-breadcrumbs/six-breadcrumbs.tsx Outdated
Comment thread libraries/ui-library/src/components/six-breadcrumbs/six-breadcrumbs.scss Outdated
Comment thread libraries/ui-library/src/components/six-breadcrumbs/six-breadcrumbs.scss Outdated
Comment thread libraries/ui-library/src/components/six-breadcrumbs/six-breadcrumbs.tsx Outdated
Comment thread libraries/ui-library/src/components/six-breadcrumbs-item/six-breadcrumbs-item.tsx Outdated
@fortesp
fortesp force-pushed the feat/add-breadcrumbs-component branch from 5c6767b to 928117c Compare June 23, 2025 08:11
@fortesp
fortesp requested review from colinscz and ibirrer June 24, 2025 06:20
@fortesp

This comment was marked as resolved.

Sign in to view
@colinscz

colinscz commented Jun 24, 2025 •
edited
Loading

Copy link
Copy Markdown
Collaborator

There are some files here i am not comfortable in pushing...

image

There have changeso on other components apart from what i am introducing with this branch... i.e. ui-library-vue...

import { defineContainer } from './vue-component-lib/utils'; change to.. import { defineContainer, type StencilVueComponent } from '@stencil/vue-output-target/runtime';

@colinscz @ibirrer whats your suggestion here... not sure why these were changed by stencil/not uptodate .. am i missing something? Should i merge only what concerns breadcrumbs?

These files are generated or re-generated upon build of the libraries by Stencil. So if you have "generated changes" in there it's totally fine, the content could differ or change based on the Stencil library versions (core, output-target etc.)
So no worries you can check them in, in fact these changes are necessary so the new components added by you are usable.

I see the Vue output is already like this on my branch which is the same as on master, so it should be fine.

@fortesp
fortesp force-pushed the feat/add-breadcrumbs-component branch from c6a85dc to cfae345 Compare June 24, 2025 11:49
ibirrer
ibirrer reviewed Jun 25, 2025
View reviewed changes

@ibirrer ibirrer left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks much better now – thanks for the improvements. I went through it once more and added a few minor comments.

Comment thread libraries/ui-library/src/components/six-breadcrumbs/six-breadcrumbs.tsx Outdated
Comment thread libraries/ui-library/src/components/six-breadcrumbs/index.html Outdated
Comment thread libraries/ui-library/src/components/six-breadcrumbs/six-breadcrumbs.tsx Outdated
Comment thread libraries/ui-library/src/components/six-breadcrumbs-item/index.html
Comment thread libraries/ui-library/src/components/six-breadcrumbs-item/six-breadcrumbs-item.tsx Outdated
Comment thread libraries/ui-library/src/components/six-breadcrumbs-item/six-breadcrumbs-item.tsx Outdated
Comment thread libraries/ui-library/src/components/six-breadcrumbs-item/six-breadcrumbs-item.tsx Outdated
@fortesp
fortesp requested a review from ibirrer July 9, 2025 11:20
@ibirrer
ibirrer force-pushed the feat/add-breadcrumbs-component branch 2 times, most recently from b31a49a to 4a5f51f Compare August 11, 2025 12:53
fortesp
fortesp commented Aug 20, 2025
View reviewed changes

@fortesp fortesp left a comment •
edited by ibirrer
Loading

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you your additions @ibirrer .
Apart from minor code style changes i do not understand why do many changes in the main file, i do not see improvement.

Comment thread libraries/ui-library/src/components/six-breadcrumbs/six-breadcrumbs.tsx Outdated
Comment thread libraries/ui-library/src/components/six-breadcrumbs/six-breadcrumbs.tsx
Comment thread libraries/ui-library/src/components/six-breadcrumbs/six-breadcrumbs.tsx
Comment thread libraries/ui-library/src/components/six-breadcrumbs-item/six-breadcrumbs-item.scss Outdated
Comment thread libraries/ui-library/src/components/six-breadcrumbs/six-breadcrumbs.scss Outdated
@ibirrer
ibirrer force-pushed the feat/add-breadcrumbs-component branch from 4a5f51f to cb3feca Compare August 22, 2025 12:34
@colinscz colinscz added this to the v5.0.0 milestone Aug 27, 2025
pennal
pennal reviewed Aug 27, 2025
View reviewed changes
Comment thread libraries/ui-library/src/components/six-breadcrumbs-item/six-breadcrumbs-item.tsx
Comment thread libraries/ui-library/src/components/six-breadcrumbs-item/six-breadcrumbs-item.tsx Outdated
Comment thread libraries/ui-library/src/components/six-breadcrumbs-item/six-breadcrumbs-item.tsx Outdated
Comment thread libraries/ui-library/src/components/six-breadcrumbs-item/six-breadcrumbs-item.tsx
Comment thread libraries/ui-library/src/components/six-breadcrumbs-item/six-breadcrumbs-item.tsx
Comment thread libraries/ui-library/src/components/six-breadcrumbs/readme.md
@ibirrer
ibirrer force-pushed the feat/add-breadcrumbs-component branch from cb3feca to c8e08e5 Compare August 28, 2025 12:36
@ibirrer

ibirrer commented Aug 28, 2025

Copy link
Copy Markdown
Collaborator

Thank you your additions @ibirrer . Apart from minor code style changes i do not understand why do many changes in the main file, i do not see improvement.

Most of the changes address bug fixes, documentation, and consistency with our other components, not just style.
See my detailed comment here: #380 (comment)

ibirrer
ibirrer previously approved these changes Sep 4, 2025
View reviewed changes
@fortesp
fortesp force-pushed the feat/add-breadcrumbs-component branch from 7f61df1 to 9f1ddd7 Compare September 9, 2025 06:16
@ibirrer
ibirrer self-requested a review September 11, 2025 07:37
ibirrer
ibirrer previously approved these changes Sep 11, 2025
View reviewed changes
colinscz
colinscz previously approved these changes Sep 11, 2025
View reviewed changes
fortesp and others added 20 commits September 12, 2025 10:56
@fortesp
docs: readme.md
b683d54
@fortesp
docs: readme.md
00be4db 
fix: six-breadcrumbs-item.scss
@fortesp
fix: six-breadcrumbs-item.scss
2051153
@fortesp
fix: removed console.log
eb33b08
@fortesp
fix: components.ts
6dc9e6f 
docs: auto gen updates
@fortesp
fix: components.ts
ee6a6e4
@fortesp
fix: components.ts
ba00750
@fortesp
fix: components.ts
e93cc11 
docs: update

# Conflicts:
#	docs/components/six-root.md
#	docs/components/six-select.md
#	docs/examples/docs-demo-six-root-0.vue
@fortesp
fix: components.ts
dbaf931 
docs: update
@fortesp
refactor: several other changes per comments
e8156fd 
fix: default separator
fix: six-click event handler
@fortesp
fix: gap between icon and the button label
e32862f 
fix: disabled replaced with read-only
fix: aria-current only on the last crumb
@fortesp
style: added index.html paragraph
a64b436
@fortesp
fix: unit tests
44dac2d
Birrer, Iwan @fortesp
chore: improve breadcrumbs component
00060e0
@fortesp
fix: Changes as per pennal comments
dac7104
@fortesp
test: Changes as per pennal comments
3af9745
@fortesp
fix: lint & prettier
814b77b
@fortesp
chore: docs
3e26723
@fortesp
chore: added examples to vue and react
51df83b
@fortesp
chore: prettier
db9f7f9
@fortesp
fortesp dismissed stale reviews from pennal, colinscz, and ibirrer via db9f7f9 September 12, 2025 08:57
@fortesp
fortesp force-pushed the feat/add-breadcrumbs-component branch from b578ab2 to db9f7f9 Compare September 12, 2025 08:57
@fortesp
fortesp merged commit b9211de into main Sep 12, 2025
1 check passed
@fortesp
fortesp deleted the feat/add-breadcrumbs-component branch September 12, 2025 10:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@colinscz colinscz colinscz approved these changes
@stefanrufer stefanrufer Awaiting requested review from stefanrufer
@pennal pennal Awaiting requested review from pennal
@ibirrer ibirrer Awaiting requested review from ibirrer

Assignees

@fortesp fortesp

Labels

None yet

Projects

None yet

Milestone

v5.0.0

Development

Successfully merging this pull request may close these issues.

5 participants

@fortesp @colinscz @ibirrer @pennal @stefanrufer